Skip to content
This repository has been archived by the owner on Feb 2, 2021. It is now read-only.

remove sha256 for docker version 1.10 #102

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

andrewsykim
Copy link

@andrewsykim andrewsykim commented Apr 15, 2016

Initially thought #93 would fix this issue but after some testing realized it does not.

When running docker images -q --no-trunc | sed 's/[^:]\+://' | sort | uniq the expected output is only the image ids but the actual result is

bash-4.3# docker images -q --no-trunc | sed 's/[^:]\+://' | sort | uniq
sha256:080defdf80d7e14d4ae2d13c00e51c1ed31103c80fafd5eba71eeae463e49c78
sha256:790bffac55a993194b4e8f7b8ea73b6e2e90891394df1bcfb2d74ce669a0bd46
sha256:7a0dbfde8b8c592e6507117b229b84a4ebc3f7c8e7dfa76b1d955c5606f51c59
sha256:87de034ee215ed4d59762748f3c9ebfa9a968f554dd00219f59ede912524d690
sha256:91c8d5fe5cb61e02957b92155f53fa3eafa4c7d2cf4a1f369b5e112f27dd10f0
sha256:b72889fa879c081224cc33d260c434ec6295b56c7677b5ff6213b5296df31aaf
sha256:f7ea1616029fbe682b17305330b7240431700863aae48e3080f314575499c7db
sha256:fa97e0a62cc44e22eb6e0057d4d04df9ffe9541e716db91724a244be8ad4535b

This fix addresses the change in output when running docker images -q --no-trunc in docker versions > 1.10. This change should not break docker-gc when running on older docker versions

bash-4.3# docker -v
Docker version 1.8.3, build f4bf5c7
bash-4.3# docker images -q --no-trunc | sed 's/sha256://' | sort | uniq
1d0b8132ce6aa7c95ed785b22593323750008d06a00dbac94b9bc670f93bba1c
37811e46c1229eec9647b29934f52e07dc387bdaeb021cb5d4e18571d54c8cb4
3dba9ec7bb63c7dc949d4282e91900ff9102ab289885c177e21e5404ca0a8876
57ae657877474a4979e2fc865ab33d52fe9f8d0611632b5a08ce093b417198c5
74e8ff6b8b6ab8bda72822a819d0dd77518f1a88177a1d744adbb10f13fb9365
774ae37038393323b5a00bb920a9b7aa25420f344648084b02857bb443f896de

@keis
Copy link
Contributor

keis commented Apr 22, 2016

You need to change the format used for images.used because that is also generated with the sha256 prefix. So right now it's trying to remove images that are in use.

Maybe it would be better to generate the exclude file in way so that it optionally matches the prefix?

@andrewsykim
Copy link
Author

andrewsykim commented Apr 22, 2016

Good catch, we shouldn't be trying to remove images used by existing containers I think I missed this because you can't force remove images if they're in use anyways.

ubuntu@testing:~$ docker rmi -f 0dc12b2
Failed to remove image (0dc12b2): Error response from daemon: conflict: unable to delete 0dc12b2 (cannot be forced) - image is being used by running container 0d322b2

Pertaining to modifying the exclude files to match the prefix how would that work? Depending on the docker version we want to ignore sha256: prepended to image ids. Generating the exlucde files to optionally match the prefix would make the code dependant on the docker version which I would personally like to avoid. Ideally the same code should work for all docker versions. Maybe here we can do this instead:

cat containers.keep |
xargs -n 1 $DOCKER inspect -f '{{.Image}}' | sed 's/sha256://' 2>/dev/null |
sort | uniq > images.used

Your thoughts?

@keis
Copy link
Contributor

keis commented Apr 22, 2016

something like this should work on both versions, also needs -E to the grep further down

     $DOCKER images \
         | tail -n+2 \
         | sed 's/^\([^ ]*\) *\([^ ]*\) *\([^ ]*\).*/ \1:\2 \3 /' \
         | grep -f $PROCESSED_EXCLUDES 2>/dev/null \
         | cut -d' ' -f3 \
-        | sed 's/^/^/' > $EXCLUDE_IDS_FILE
+        | sed 's/^/^(sha256:)?/' > $EXCLUDE_IDS_FILE

But I'm not really sure what I prefer. One issue I see with stripping the sha256 prefix is that there probably more places where a format like this will be produce and it could be hard to know we're not missing a spot.

Anyway I think this is up to the maintainers to decide I'm just another user seeing the same bug :)

@sfrique
Copy link
Contributor

sfrique commented Jun 17, 2016

While I was using it, i thought of the same solution as @keis

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants